Refactor Tokens contract #518
Conversation
src/cc/CCcontract.h
Outdated
| /*** | ||
| * Default ctor, sets everything to 0 | ||
| */ | ||
| CCcontract_info() { memset(this, 0, sizeof(CCcontract_info)); } |
There was a problem hiding this comment.
Here the memset function is used to nullify fields of CCcontract_info class. Virtual table pointer (vptr) could be damaged, bcz memset doesn't know anything about vptr and sizeof(CCcontract_info) is virtual methods existence dependent. May be such default ctor with member initializer list will be more correct?
struct CCcontract_info
{
uint8_t evalcode;
uint8_t additionalTokensEvalcode2 = 0;
char unspendableCCaddr[64];
// < members skipped >
CCcontract_info(): evalcode(0), additionalTokensEvalcode2(0),
unspendableCCaddr{0} /*, etc. */
{}
virtual ~CCcontract_info() {}
// < virtual methods skipped >
};
There was a problem hiding this comment.
yes i'd prefer to avoid using memset with c++ objects, as it may override members' constructor initialisations
|
found one catch let's provide py tests to work, this will make life easier |
I thought I took care of that, but I'll research all of these issues. Thanks @DeckerSU and @dimxy for the reviews. |
|
I was certainly calling the incorrect method for MakeCC1vout. I believe that has been fixed. I will also attempt to build a test to prove that it is fixed. As for the vtable issue, I was certainly stepping on the pointer. Fixing this requires a sizable adjustment. As currently implemented, CCcontract_info needs to be in the common lib in order for the virtual method implementations to link correctly. That comes with a lot of unintended side effects that I want to avoid. I am taking a step back to see what other options are available. Update: vtable issue fixed. Solution was to use pure virtual on 1 method that is always overridden, and not virtualize a method that never is overriden. Embarrassingly obvious. |
jmjatlanta
force-pushed
the
jmj_cctokens_refactor
branch
from
36564c4 to
2bcc883
Compare
|
As this affects a CC contract, it has been suggested (and I agree) that such changes not be done on the For the moment, I will leave this open, but I suggest that review of this PR be a (very) low priority. |
|
This PR should be retargeted to the 'research' branch (where cc code is developed first) |
NOTE 1: This builds on the jmj_ccinit branch. It will make for easier review if PR #444 was reviewed first.
NOTE 2: There have undoubtedly been a number of logic changes to the tokens contract as part of the Tokel project, Merging this into those changes will probably be difficult. If the ideas in this PR are desirable, it may be best to replay these commits after the Tokel changes are merged in (or redo this PR manually).
The functionality of the CC Tokens code was spread within several files. This PR moves them into a few, and wraps then into a class. In some cases, this eliminated the need to pass the CCcontract_info object as a parameter. In other cases, this required the call to static methods within the class.
NOTE 3: the function CCTokens::GetCCaddress is an example where converting this from a pseudo-override type function of (::GetCCaddress) to a template would avoid the runtime hit of indirection as well as increase type safety by removal of a few dynamic_casts, in exchange for increased compile time. ::GetCCaddress may be a candidate for something that should be within the CCcontract_info class, but that has not been explored.